drivers: sensor: Add support for BH1730 ambient light sensor #89908
Conversation
|
Hello @rtborg, and thank you very much for your first pull request to the Zephyr project! |
github-actions
bot
requested review from
asemjonovs,
avisconti,
jeppenodgaard,
MaureenHelm,
teburd,
tristan-google,
ubieda and
yperess
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the BH1730 ambient light sensor by introducing a new sensor driver, device tree bindings, and associated build changes.
- Added a new sensor node in the DTS test file.
- Introduced DTS binding YAML for the BH1730 sensor.
- Implemented the sensor driver with configuration (Kconfig, CMakeLists.txt) and driver source code.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/drivers/build_all/sensor/i2c.dtsi | Added sensor node for BH1730. |
| dts/bindings/sensor/rohm,bh1730.yaml | New DTS binding file describing sensor properties and defaults. |
| drivers/sensor/rohm/bh1730/bh1730.c | New driver code implementing sensor functionality. |
| drivers/sensor/rohm/bh1730/Kconfig | Added configuration options for the BH1730 sensor. |
| drivers/sensor/rohm/bh1730/CMakeLists.txt | Build file listing new driver source. |
| drivers/sensor/rohm/Kconfig | Updated to include the new sensor driver Kconfig. |
| drivers/sensor/rohm/CMakeLists.txt | Updated to add the BH1730 subdirectory when enabled. |
Comments suppressed due to low confidence (1)
drivers/sensor/rohm/bh1730/bh1730.c:29
- The constant 'BH1730_CONTORL_ADC_EN_POWER_ON_SINGLE_READING' is misspelled; consider renaming it to 'BH1730_CONTROL_ADC_EN_POWER_ON_SINGLE_READING' for clarity.
#define BH1730_CONTORL_ADC_EN_POWER_ON_SINGLE_READING 0x0B
rtborg
force-pushed
the
bh1730_sensor
branch
2 times, most recently
from
9e03ed6 to
9f7d049
Compare
rtborg
force-pushed
the
bh1730_sensor
branch
2 times, most recently
from
0e381c6 to
37abf2c
Compare
|
Are there any other comments which need addressing? |
|
Comments fixed now |
rtborg
force-pushed
the
bh1730_sensor
branch
2 times, most recently
from
85bb848 to
242477c
Compare
This commit adds support for BH1730 ambient light sensor. Signed-off-by: Borislav Kereziev <b.kereziev@gmail.com>
|
|
Hi @rtborg! To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge. Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁 |
| #define BH1730_CONTROL_ADC_EN_POWER_ON_SINGLE_READING 0x0B | ||
| #define BH1730_CONTROL_ADC_EN_POWER_ON 0x03 | ||
| #define BH1730_CONTROL_ADC_DATA_UPDATED (1 << 4) | ||
| #define BH1730_TINT 2.8E-9 /* Internal clock period Tint */ |
There was a problem hiding this comment.
This doesn't appear to be used.
| if (gain_reg_val == 0x0) { | ||
| return 1; | ||
| } | ||
| if (gain_reg_val == 0x01) { | ||
| return 2; | ||
| } | ||
| if (gain_reg_val == 0x02) { | ||
| return 64; | ||
| } | ||
| if (gain_reg_val == 0x03) { | ||
| return 128; | ||
| } | ||
|
|
||
| return 1; |
There was a problem hiding this comment.
nit, this is a bit better (fewer branches):
static uint8_t lookup_table = [1, 2, 64, 128];
if (gain_reg_val >= ARRAY_SIZE(lookup_table)) {
return 1;
}
return lookup_table[gain_reg_val];
|
thanks for adding this to Zephyr 🙏 |
This commit adds support for BH1730 ambient light sensor.